feat(ui): Mosaic field component - #9322
Conversation
🦋 Changeset detectedLatest commit: 1bcc702 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-google-signin
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
57aeff6 to
fd49235
Compare
ca96e1b to
a6c399c
Compare
ee83a6e to
fcbd583
Compare
fcbd583 to
374b29c
Compare
b387d17 to
e014d4e
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdded the Mosaic Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
packages/ui/src/mosaic/components/field/index.ts (1)
1-2: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the new nested barrel file.
The new
packages/ui/src/mosaic/components/field/index.tsre-export file conflicts with the repository rule that prohibits barrel files because they can create circular dependencies.
packages/ui/src/mosaic/components/field/index.ts#L1-L2: remove this barrel file.packages/ui/src/mosaic/styles/index.ts#L17-L18: re-exportFieldand its prop types directly from../components/field/field.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/mosaic/components/field/index.ts` around lines 1 - 2, Remove the nested barrel file packages/ui/src/mosaic/components/field/index.ts (lines 1-2). Update the exports at packages/ui/src/mosaic/styles/index.ts (lines 17-18) to re-export Field and its prop types directly from ../components/field/field.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.changeset/quiet-fields-compose.md:
- Around line 1-2: Replace the empty changeset frontmatter in
quiet-fields-compose.md with an `@clerk/ui` minor release entry, and add a concise
description of the new public Field compound component API.
In `@packages/swingset/src/stories/field.component.mdx`:
- Line 36: Update the Field description near the Field.Root usage note to remove
the claim that it does not propagate semantic state, while preserving the
statements that it does not validate controls or render errors automatically and
that its parts can be used independently.
In `@packages/swingset/src/stories/field.component.stories.tsx`:
- Line 1: Add the Emotion JSX import-source pragma at the beginning of the field
component story, before the existing Field import, so the styled Mosaic story
uses Emotion's JSX transform.
In `@packages/ui/src/mosaic/components/field/field.context.tsx`:
- Around line 4-10: Propagate disabled, required, and invalid state from
Field.Root to composed controls: in
packages/ui/src/mosaic/components/field/field.context.tsx lines 4-10, add typed
state fields to FieldContextValue; in
packages/ui/src/mosaic/components/field/field.tsx lines 20-37, add the states to
FieldRootProps and pass them through FieldProvider; in
packages/ui/src/mosaic/components/input/input.tsx lines 15-39, resolve explicit
control props before context values and apply the resulting disabled, required,
and aria-invalid attributes.
- Around line 67-74: Update the field context around the useSafeLayoutEffect
registration and the input relationship handling in
packages/ui/src/mosaic/components/field/field.context.tsx:67-74 and
packages/ui/src/mosaic/components/input/input.tsx:37-39 so generated Description
and Error IDs are derived during render rather than only populated by the
effect, while preserving client-side registration and cleanup. Add an SSR
assertion covering the generated aria-describedby relationships.
In `@packages/ui/src/mosaic/components/field/field.ssr.test.tsx`:
- Around line 48-62: Update Field.Root and its associated label/control ID
handling so an explicit Input id such as custom-control is preserved in SSR
markup and used for the label for attribute. Update
packages/ui/src/mosaic/components/field/field.ssr.test.tsx lines 48-62 to expect
custom-control in both locations, and update
packages/ui/src/mosaic/components/field/field.test.tsx lines 147-176 to verify
hydration preserves the server-rendered ID without adding it afterward.
---
Nitpick comments:
In `@packages/ui/src/mosaic/components/field/index.ts`:
- Around line 1-2: Remove the nested barrel file
packages/ui/src/mosaic/components/field/index.ts (lines 1-2). Update the exports
at packages/ui/src/mosaic/styles/index.ts (lines 17-18) to re-export Field and
its prop types directly from ../components/field/field.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: beecab09-0ad7-4c52-8e49-9ae6062c9e24
📒 Files selected for processing (14)
.changeset/quiet-fields-compose.mdpackages/swingset/src/components/DocsViewer.tsxpackages/swingset/src/lib/registry.tspackages/swingset/src/stories/field.component.mdxpackages/swingset/src/stories/field.component.stories.tsxpackages/ui/src/mosaic/components/field/field.context.tsxpackages/ui/src/mosaic/components/field/field.ssr.test.tsxpackages/ui/src/mosaic/components/field/field.styles.tspackages/ui/src/mosaic/components/field/field.test.tsxpackages/ui/src/mosaic/components/field/field.tsxpackages/ui/src/mosaic/components/field/index.tspackages/ui/src/mosaic/components/input/input.tsxpackages/ui/src/mosaic/icons/registry.tsxpackages/ui/src/mosaic/styles/index.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)clerk/clerk-ios(auto-detected)clerk/clerk-android(auto-detected)clerk/cli(auto-detected)
e014d4e to
cfe215d
Compare
cfe215d to
1ef2b96
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ui/src/mosaic/components/field/field.context.tsx (1)
117-136: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftPreserve caller-provided control IDs during SSR.
Line 122 registers the supplied
idonly after commit. During SSR,context.controlIdremainsdefaultControlId.Inputthen uses that value before its ownid, so<Input id="email" />renders with a generated ID and changes after hydration.Derive the initial control ID during render, or provide it to
FieldProviderbefore descendant rendering. Add an SSR test that verifies the inputidand generated labelhtmlForboth use a caller-provided ID.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/mosaic/components/field/field.context.tsx` around lines 117 - 136, Update the control-ID derivation in the field context render path so a caller-provided id is available during SSR, instead of relying only on the post-commit registerControlId effect. Ensure descendants such as Input and generated labels use that initial ID consistently, while preserving generated IDs when no id is supplied. Add an SSR test covering both the rendered input id and label htmlFor for a caller-provided ID.
🧹 Nitpick comments (1)
packages/ui/src/mosaic/components/field/field.tsx (1)
21-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the exported props interface.
Add concise JSDoc for
FieldRootProps. Document that its state props apply to composed controls through field context.As per coding guidelines, “All public APIs must be documented with JSDoc.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/mosaic/components/field/field.tsx` around lines 21 - 25, Add concise JSDoc immediately above the exported FieldRootProps interface, documenting the interface and clarifying that its disabled, required, and invalid state props apply to composed controls through field context.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/ui/src/mosaic/components/field/field.context.tsx`:
- Around line 117-136: Update the control-ID derivation in the field context
render path so a caller-provided id is available during SSR, instead of relying
only on the post-commit registerControlId effect. Ensure descendants such as
Input and generated labels use that initial ID consistently, while preserving
generated IDs when no id is supplied. Add an SSR test covering both the rendered
input id and label htmlFor for a caller-provided ID.
---
Nitpick comments:
In `@packages/ui/src/mosaic/components/field/field.tsx`:
- Around line 21-25: Add concise JSDoc immediately above the exported
FieldRootProps interface, documenting the interface and clarifying that its
disabled, required, and invalid state props apply to composed controls through
field context.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: f801a372-56ed-4ed9-9d67-a21a6ba88cac
📒 Files selected for processing (5)
packages/swingset/src/stories/field.component.mdxpackages/ui/src/mosaic/components/field/field.context.tsxpackages/ui/src/mosaic/components/field/field.test.tsxpackages/ui/src/mosaic/components/field/field.tsxpackages/ui/src/mosaic/components/input/input.tsx
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)clerk/cli(auto-detected)clerk/clerk-ios(auto-detected)clerk/clerk-android(auto-detected)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/swingset/src/stories/field.component.mdx
- packages/ui/src/mosaic/components/field/field.test.tsx
- packages/ui/src/mosaic/components/input/input.tsx
Description
https://swingset-git-mosaic-text-field-component.clerkstage.dev/components/field
Adds the Mosaic Field component:
Field.Root,Field.Label,Field.Description, andField.ErrorAPI.aria-*attributes.invalid,disabled, andrequiredto composed Mosaic inputs.alert-circleicon. Validation, error visibility, and lifecycle state such asdirty,touched,filled, andfocusedremain caller-owned.